Stop restarting pgadmin on every lerd start - #1494
Conversation
|
Appreciate you picking this one up, it's a genuinely annoying thing to chase. I dug into it on my machine though and I don't think it breaks the loop. MaterializeServiceFiles is already a one line wrapper around MaterializeServiceFilesChanged, so the seam swap doesn't change what gets written, and the early return never fires for pgadmin because changed comes back true on every pass. The reason is easy to miss from the outside. The pgpass mount is mode 0600 with chown true, so podman's :U re-owns it to a userns mapped uid on container start and the host user can't read it back afterwards. The content compare does an os.ReadFile, gets EACCES, and drops through to the unlink and rewrite branch, which sets changed. On my install the file sits there owned by 105049 at 0600 and cat gives permission denied, and re-materialising over it moves the mtime every time. So the loop is the file being unreadable, not the content differing, and you'd have to be looking right at it to spot that. Flagging the test too, since it can't fail as written. It stubs the materialise seam to return false, a state pgadmin never actually produces, so it ends up asserting the patch instead of the bug and would stay green on a build that still restarts on every start. The early return also costs a little even where it does work. The mtime is the durable signal and changed isn't, so if a pass rewrites a file and the restart then fails, the next pass bails before the mtime check ever runs and the container keeps serving stale config. Whatever fixes this needs something we can still read after podman takes the file, probably a sidecar holding a hash of the last rendered content compared against the freshly rendered bytes. Happy for you to take another swing at it if you're up for it, otherwise I'll pick it up from here. |
podman's :U flag re-owns the pgpass file to a userns-mapped uid, so os.ReadFile EACCES and MaterializeServiceFilesChanged rewrites it on every pass, moving the mtime past the container's boot. The next lerd start sees it as drifted and restarts again, looping forever. Store a sha256 sidecar beside each materialised file. The sidecar is never mounted into the container so podman never re-owns it, and the host user can always read it back. When ReadFile fails, compare the sidecar hash instead; if it matches, skip the rewrite so the mtime does not move.
9960d11 to
f796edb
Compare
|
i updated with the sidecar hash approach you suggested |
The hash is only needed for a mount podman re-owns through :U, where a restrictive mode leaves the file unreadable and the content compare cannot run at all. Writing one beside every preset file doubles the entries in a service's directory for nothing, so it now follows chown and is rewritten only when it actually moves. Covers the other half of it too, a changed rendering still has to reach a file we cannot read back, and puts back the rationale the comments carried for comparing by content and for unlinking before the rewrite.
|
Verified this one on a real install with pgadmin reinstalled, and it holds. With the released binary the container restarted and pgpass got rewritten a couple of seconds after it, which is the loop. With your build the first start restarted once to clear the drift the old binary had just left behind, and the two starts after that moved nothing at all, container start time and pgpass mtime both frozen. I pushed one commit on top rather than send you round again. The hash now follows chown, since only a mount podman re-owns can ever go unreadable and every other preset file was picking up a .sha256 next to it for nothing, and it is written only when it actually changes. I also added the other half of the test, a changed rendering still has to reach a file we cannot read back, and put the two comment blocks back where they explained why we compare by content and why the rewrite unlinks first. Good instinct on the sidecar, it is the right shape for this. Thanks for sticking with it. |
|
thanks for the updated George |
2 participants
RestartIfConfigDrifted rewrote a service's preset config files unconditionally and restarted the container when the newest file was newer than the container's boot. For pgadmin the pgpass file is rewritten on every pass even when its content has not changed, so its mtime advances past the container's start and the next lerd start sees it as drifted and restarts again, looping forever.
MaterializeServiceFilesChanged already compares content and skips the rewrite when nothing differs, so the mtime does not move. Route RestartIfConfigDrifted through it and bail out early when it reports no change.
Closes #1490